perf(memtrack): bound ring reads and resume paused pids at low fill - #549
Conversation
Merging this PR will not alter performance
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| while ringbuf.consume_raw_n(CONSUME_CHUNK_RECORDS) | ||
| == CONSUME_CHUNK_RECORDS as i32 | ||
| { | ||
| release_if_low(); | ||
| } |
There was a problem hiding this comment.
drain() while processes are still running, but its request cannot be acknowledged until the loop ends, so it can block indefinitely. Limit the work per tick or check for control requests between chunks.
Knowledge Base Used: eBPF memory tracker
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/memtrack/src/ebpf/poller.rs
Line: 145-149
Comment:
**Full chunks delay drain requests** If producers keep supplying at least 1,024 records per read, this loop keeps consuming without checking the control channel. The attach worker can request a synchronous `drain()` while processes are still running, but its request cannot be acknowledged until the loop ends, so it can block indefinitely. Limit the work per tick or check for control requests between chunks.
**Knowledge Base Used:** [eBPF memory tracker](https://app.greptile.com/codspeed/-/custom-context/knowledge-base/codspeedhq/codspeed/-/docs/ebpf-memory-tracker.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
415571b to
4c24a9a
Compare
A regular poll tick drained the whole ring in one `poll(ZERO)`, and pressure-stopped processes were resumed only after that tick ended with the ring completely empty. On a large memory benchmark suite with stack capture, single ticks of the stacks ring ran 0.4-1.7 s, every pressure stop landed inside one, and stopped processes waited 238-513 ms (median) to resume, 5-7% of the run. Ticks now read in chunks of 1024 records with `consume_raw_n` and check the fill between chunks. Paused processes resume once the ring is below a quarter full instead of empty; BPF stops them at three quarters, so the two thresholds leave room between stop and resume. `drain()` still reads the ring fully, and shutdown still releases unconditionally. Closes COD-3659
4c24a9a to
96b0517
Compare
1 participant
Bound stacks-ring reads and resume pressure-stopped processes at low fill.
A regular poll tick drained the whole ring in one
poll(Duration::ZERO), and pressure-stopped processes were only resumed after that tick ended with the ring completely empty. Measured on a large memory benchmark suite with stack capture (CODSPEED_MEMTRACK_STATS, #546):Changes:
RingBuffer::consume_raw_nand check the fill between chunks.on_drained→on_low_fill) instead of at empty. BPF stops at 3/4, so the two thresholds leave room between stop and resume.drain()still reads the ring fully, and shutdown still releases unconditionally.Review note:
release_pressurenow also runs between chunks while the ring is below the watermark, which is onepressure_stoppedkey iteration per 1024 records on a busy ring.pressure_tests.rs(BPF, root) is not in the CIbpf-testsmatrix, so I ran it by hand onubuntu-latestandubuntu-24.04-arm, on this branch and on its base. I usedITERATIONS= 50000: at the committed 400000, the slow-poller phase was canceled on hosted runners both on this branch and on the base, most likely memory (the test reads the event channel only after the fixture exits). Both tests pass on both branches withdropped 0. Wall time of the paused runs (x86 / arm):slow_poller_pause_recovers_without_loss(slow poller)slow_poller_pause_resumes_every_writing_processWith the test's 10 s poll, base resumes paused processes once per tick (about every 10.6 s); this branch resumes them between chunks (about every 0.9 s).
Closes COD-3659